fix(web): filter system-injected XML tags from rendering as raw text#387
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
- [Major] Generic XML filtering now rewrites or drops legitimate human prompts across non-Claude sessions, because
reduceTimeline()applies the Claude-specific tag heuristic to everyrole: 'user'message. Codex and Gemini local sessions both forward human input throughsendUserMessage()as plain user text (cli/src/api/apiSession.ts:407,cli/src/codex/session.ts:87,cli/src/gemini/session.ts:93), so a prompt that starts with<system-reminder>disappears entirely and<task-notification>...<summary>...</summary>is rendered as an agent event instead of the user’s text. Evidence:web/src/chat/reducerTimeline.ts:75.
Summary
Review mode: initial
One major regression found in web/src/chat/reducerTimeline.ts: Claude-specific XML filtering was added at the generic timeline layer, so it now affects normal user messages for other agent flavors as well.
Testing
Not run (automation; bun is unavailable in this runner, so the added Vitest file could not be executed here)
HAPI Bot
web/src/chat/reducerTimeline.ts
Outdated
| }) | ||
| continue | ||
| } | ||
| if (isSystemInjectedMessage(msg.content.text)) { |
There was a problem hiding this comment.
[MAJOR] This runs in the generic msg.role === 'user' path, so it also rewrites or drops real human prompts for non-Claude sessions. sendUserMessage() still forwards Codex/Gemini user input as plain role: 'user' text (cli/src/api/apiSession.ts:407, cli/src/codex/session.ts:87, cli/src/gemini/session.ts:93), which means a prompt starting with <system-reminder> now vanishes and a prompt starting with <task-notification> becomes an agent-event.
Suggested fix:
// Keep Claude transcript cleanup in normalizeUserOutput(), which only runs for
// Claude JSONL `output` payloads, instead of filtering every user message here.
if (trimmed.startsWith('<task-notification>')) {
const summary = trimmed.match(/<summary>([\s\S]*?)<\/summary>/)?.[1]?.trim()
if (!summary) return null
return {
id: messageId,
localId,
createdAt,
role: 'event',
content: { type: 'message', message: summary },
isSidechain: false,
meta
}
}
if (
trimmed.startsWith('<system-reminder>') ||
trimmed.startsWith('<command-name>') ||
trimmed.startsWith('<local-command-caveat>')
) {
return null
}
There was a problem hiding this comment.
Findings
- [Major]
<task-notification>conversion is still unreachable on the real Claude path, because injected XML is stored asrole: 'agent'/type: 'output'insendClaudeSessionMessage()andnormalizeUserOutput()still returnsnullfor<task-notification>beforereduceTimeline()sees amsg.role === 'user'. The new reducer branch therefore only handles synthetic inputs like the new test, so the UI will keep silently dropping task notifications instead of rendering the promised agent event. Evidenceweb/src/chat/reducerTimeline.ts:77, related contextweb/src/chat/normalizeAgent.ts:117,cli/src/api/apiSession.ts:364,web/src/chat/reducerTimeline.test.ts:29.
Suggested fix:if (typeof messageContent === 'string') { const trimmed = messageContent.trimStart() if (trimmed.startsWith('<task-notification>')) { const summary = trimmed.match(/<summary>([\s\S]*?)<\/summary>/)?.[1]?.trim() if (!summary) return null return { id: messageId, localId, createdAt, role: 'event', content: { type: 'message', message: summary }, isSidechain: false, meta } } if ( trimmed.startsWith('<system-reminder>') || trimmed.startsWith('<command-name>') || trimmed.startsWith('<local-command-caveat>') ) { return null } }
Summary
Review mode: follow-up after new commits
One major issue remains in the follow-up diff: the new reducer logic does not run on the actual Claude transcript shape for <task-notification>, so the user-visible regression is only partially fixed.
Testing
Not run (automation; bun is not installed in this runner)
HAPI Bot
web/src/chat/reducerTimeline.ts
Outdated
|
|
||
| if (msg.role === 'user') { | ||
| if (context.isClaudeSession) { | ||
| const taskSummary = parseTaskNotificationSummary(msg.content.text) |
There was a problem hiding this comment.
[MAJOR] parseTaskNotificationSummary() is wired into the msg.role === 'user' reducer path, but real Claude injections never reach this branch. sendClaudeSessionMessage() stores non-external Claude XML as role: 'agent' / type: 'output' (cli/src/api/apiSession.ts:364), and normalizeUserOutput() still returns null for <task-notification> before reduction (web/src/chat/normalizeAgent.ts:117). That means the new agent-event behavior is unreachable in production, while the added test only covers a synthetic traced role: 'user' message (web/src/chat/reducerTimeline.test.ts:29).
Suggested fix:
if (typeof messageContent === 'string') {
const trimmed = messageContent.trimStart()
if (trimmed.startsWith('<task-notification>')) {
const summary = trimmed.match(/<summary>([\s\S]*?)<\/summary>/)?.[1]?.trim()
if (!summary) return null
return {
id: messageId,
localId,
createdAt,
role: 'event',
content: { type: 'message', message: summary },
isSidechain: false,
meta
}
}
if (
trimmed.startsWith('<system-reminder>') ||
trimmed.startsWith('<command-name>') ||
trimmed.startsWith('<local-command-caveat>')
) {
return null
}
}
|
Thanks for the follow-up review! Regarding the second finding — I've traced the full message flow and believe the current implementation is correct:
The comment in The |
There was a problem hiding this comment.
Review mode: follow-up after new commits
Findings
- [Major] The new
<task-notification>handling innormalizeUserOutput()is still unreachable.web/src/chat/normalizeAgent.ts:114returnsnullfor<task-notification>and<system-reminder>before the added conversion/sidechain branch atweb/src/chat/normalizeAgent.ts:137runs, so the new expectations inweb/src/chat/normalize.test.ts:108andweb/src/chat/normalize.test.ts:178cannot pass. In production this still drops Claude task notifications instead of rendering the promised event. - [Minor] The new Claude-only fallback in
web/src/chat/reducerTimeline.ts:75runs on every normalizedrole: 'user'message, including genuine user input fromweb/src/chat/normalizeUser.ts:37and optimistic sends inweb/src/hooks/mutations/useSendMessage.ts:64. If a user pastes one of these XML tags into chat, their own message is silently dropped or rewritten.
Summary
- 1 major issue remains in the follow-up patch, plus 1 narrower regression in the reducer fallback for real user messages.
Testing
- Not run (automation;
bunis unavailable in this runner)
HAPI Bot
| isSidechain: false, | ||
| meta | ||
| } | ||
| } |
There was a problem hiding this comment.
[MAJOR] The new <task-notification> conversion here is still dead code because the old guard above it returns null for both <task-notification> and <system-reminder> first (web/src/chat/normalizeAgent.ts:114). That means the new expectations in web/src/chat/normalize.test.ts:108 / web/src/chat/normalize.test.ts:178 cannot pass, and Claude task notifications will still be dropped instead of rendered as role: 'event'.
Suggested fix:
if (typeof messageContent === 'string') {
const trimmed = messageContent.trimStart()
if (trimmed.startsWith('<task-notification>')) {
const summary = trimmed.match(/<summary>([\s\S]*?)<\/summary>/)?.[1]?.trim()
return summary ? {
id: messageId,
localId,
createdAt,
role: 'event',
content: { type: 'message', message: summary },
isSidechain: false,
meta
} : {
id: messageId,
localId,
createdAt,
role: 'agent',
isSidechain: true,
content: [{ type: 'sidechain', uuid, prompt: messageContent }]
}
}
}
web/src/chat/reducerTimeline.ts
Outdated
| } | ||
|
|
||
| if (msg.role === 'user') { | ||
| if (context.isClaudeSession) { |
There was a problem hiding this comment.
[MINOR] This new reducer branch runs on every normalized role: 'user' message in Claude sessions, not just injected XML. normalizeUserRecord() turns genuine human text into role: 'user' (web/src/chat/normalizeUser.ts:37), and optimistic sends use the same shape (web/src/hooks/mutations/useSendMessage.ts:64), so a user who pastes <task-notification>...</task-notification> or <system-reminder>...</system-reminder> into chat will lose or mutate their own message.
Suggested fix:
if (msg.role === 'user') {
// Do not filter real user messages here.
// Keep Claude XML handling in normalizeUserOutput(), where injected
// type:'user' agent payloads are distinguishable from human input.
}
Claude Code injects internal messages (<task-notification>, <system-reminder>, <command-name>, <local-command-caveat>) as user-role messages. The web UI was rendering these as raw XML text visible to users. - Parse <task-notification> and display as agent-event with summary text - Silently drop <system-reminder>, <command-name>, <local-command-caveat> - Add tests covering all injection prefixes and edge cases
Address review feedback: the XML tag filtering was applied at the generic timeline layer, which could incorrectly hide legitimate user messages in Codex/Gemini sessions. - Add isClaudeSession flag threaded from Session.metadata.claudeSessionId - Only filter system-injected tags when isClaudeSession is true - Add tests verifying non-Claude sessions pass through all messages
… leaks Restores the fix from 3cf96ab that was accidentally reverted in 2205e04. In normalizeUserOutput(), string-content user messages arriving through the agent output path are never real user input (real user text goes through normalizeUserRecord). Previously, non-sidechain string messages were emitted as role:'user', causing subagent prompts and system-injected messages to render as user text in the web UI. Now all string-content user messages in this path are: - <task-notification> with summary → converted to role:'event' - Everything else → marked as sidechain (matched to parent Task tool call by the tracer, or harmlessly skipped by the reducer) This provides a root-level fix that prevents ANY string user message from the agent output path from leaking as visible user text.
There was a problem hiding this comment.
Review mode: follow-up after new commits
Findings
- [Major] The new
<task-notification>path is still unreachable becausenormalizeUserOutput()returnsnullfor the same tag earlier, so real Claude task notifications never reach the reducer fallback. Evidenceweb/src/chat/normalizeAgent.ts:114,web/src/chat/normalizeAgent.ts:140,web/src/chat/reducerTimeline.ts:75. - [Minor] The new Claude-only reducer filter now runs on every normalized human
role: 'user'message, so a user who pastes one of these XML tags will have their own message dropped or rewritten. Evidenceweb/src/chat/reducerTimeline.ts:75,web/src/chat/normalizeUser.ts:37,web/src/hooks/mutations/useSendMessage.ts:64.
Summary
- Review mode: follow-up after new commits
- 1 major issue remains, plus 1 minor regression in the new reducer fallback.
Testing
- Not run (automation)
HAPI Bot
| isSidechain: false, | ||
| meta | ||
| } | ||
| } |
There was a problem hiding this comment.
[MAJOR] This branch is still dead on the current head. normalizeUserOutput() returns null for <task-notification> a few lines earlier at web/src/chat/normalizeAgent.ts:114, so real Claude injections never reach this conversion and the new reducer fallback only affects synthetic role: 'user' messages from tests.
Suggested fix:
if (typeof messageContent === 'string') {
const trimmed = messageContent.trimStart()
if (trimmed.startsWith('<task-notification>')) {
const summary = trimmed.match(/<summary>([\s\S]*?)<\/summary>/)?.[1]?.trim()
return summary
? {
id: messageId,
localId,
createdAt,
role: 'event',
content: { type: 'message', message: summary },
isSidechain: false,
meta
}
: {
id: messageId,
localId,
createdAt,
role: 'agent',
isSidechain: true,
content: [{ type: 'sidechain', uuid, prompt: messageContent }]
}
}
}
web/src/chat/reducerTimeline.ts
Outdated
| } | ||
|
|
||
| if (msg.role === 'user') { | ||
| if (context.isClaudeSession) { |
There was a problem hiding this comment.
[MINOR] This new branch runs on every normalized role: 'user' message in Claude sessions, including real human input from normalizeUserRecord() and optimistic sends. If a user pastes <task-notification>...</task-notification> or <system-reminder>...</system-reminder>, their own message is silently dropped or rewritten.
Suggested fix:
if (msg.role === 'user') {
if (isCliOutputText(msg.content.text, msg.meta)) {
// existing CLI-output handling
}
blocks.push({
kind: 'user-text',
id: msg.id,
localId: msg.localId,
createdAt: msg.createdAt,
text: msg.content.text,
attachments: msg.content.attachments,
status: msg.status,
originalText: msg.originalText,
meta: msg.meta
})
continue
}
hqhq1025
force-pushed
the
fix/web-filter-system-injected-messages
branch
from
488dd6d to
5101e90
Compare
There was a problem hiding this comment.
Findings
- [Minor] Reducer-side Claude XML filtering still runs on genuine user messages, so a user who pastes one of these tags will have their own text hidden or rewritten. Agent-path injections are already normalized earlier in
normalizeUserOutput(), while persisted and optimistic human messages still come throughnormalizeUserRecord()asrole: 'user'. Evidenceweb/src/chat/reducerTimeline.ts:75,web/src/chat/normalizeAgent.ts:123,web/src/chat/normalize.ts:23,web/src/chat/normalizeUser.ts:37.
Suggested fix:if (msg.role === 'user') { if (isCliOutputText(msg.content.text, msg.meta)) { // existing CLI-output handling } blocks.push({ kind: 'user-text', id: msg.id, localId: msg.localId, createdAt: msg.createdAt, text: msg.content.text, attachments: msg.content.attachments, status: msg.status, originalText: msg.originalText, meta: msg.meta }) continue }
Summary
Review mode: follow-up after new commits
- 1 minor regression remains in the new reducer fallback; the earlier unreachable-branch issue is fixed on this head.
Testing
- Not run (automation;
bunnot available in runner)
HAPI Bot
web/src/chat/reducerTimeline.ts
Outdated
| } | ||
|
|
||
| if (msg.role === 'user') { | ||
| if (context.isClaudeSession) { |
There was a problem hiding this comment.
[MINOR] This Claude-only reducer fallback still runs on every normalized role: user message. Real human input from normalizeUserRecord() and optimistic sends already lands on this path, while injected type: "user" agent output is already handled earlier in normalizeUserOutput(). A Claude user who pastes <system-reminder>...</system-reminder> or <task-notification>...</task-notification> will have their own message dropped or rewritten.
Suggested fix:
if (msg.role === user) {
if (isCliOutputText(msg.content.text, msg.meta)) {
// existing CLI-output handling
}
blocks.push({
kind: user-text,
id: msg.id,
localId: msg.localId,
createdAt: msg.createdAt,
text: msg.content.text,
attachments: msg.content.attachments,
status: msg.status,
originalText: msg.originalText,
meta: msg.meta
})
continue
}System-injected messages are now fully handled in normalizeUserOutput() (normalize layer), so the redundant filtering in reduceTimeline() is no longer needed. Removing it also eliminates the risk of accidentally hiding legitimate user messages that happen to start with XML tags. - Remove SYSTEM_INJECTION_PREFIXES, isSystemInjectedMessage, parseTaskNotificationSummary from reducerTimeline.ts - Remove isClaudeSession plumbing from reducer.ts and SessionChat.tsx - Simplify reducerTimeline.test.ts to only test pass-through behavior
There was a problem hiding this comment.
Findings
- No issues found in added/modified lines.
Summary
Review mode: follow-up after new commits
- No new correctness, security, or regression issues found in the latest diff.
- Residual risk: the normalize-layer filtering added here is only directly covered for
<task-notification>and<system-reminder>in the updated tests;<command-name>,<local-command-caveat>, leading whitespace, and empty-summary cases are still untested on this head.
Testing
- Not run (automation;
bunnot available in runner)
HAPI Bot
2 participants
Summary
<task-notification>,<system-reminder>,<command-name>,<local-command-caveat>) as user-role messages for model context. The web UI was rendering these as raw XML text visible to users.<task-notification>is now parsed and displayed as anagent-eventblock showing the summary text (e.g., "Background command X was stopped")<system-reminder>,<command-name>, and<local-command-caveat>are silently droppedSYSTEM_INJECTION_PREFIXESlist fromcli/src/api/apiSession.tsto keep filtering in syncTest plan
reducerTimeline.test.tswith 8 tests covering:<task-notification>with summary → agent-event<task-notification>without summary → silently dropped<task-notification>with empty<summary></summary>→ silently dropped<task-notification>with leading whitespace → handled correctly<system-reminder>→ hidden<command-name>→ hidden<local-command-caveat>→ hiddenbun run typecheckpasses